Skip to content

Fix Sign in with Google for accounts using the WordPress Two-Factor plugin#13118

Open
shervElmi wants to merge 10 commits into
developfrom
bug/11032-siwg-two-factor-account-handling
Open

Fix Sign in with Google for accounts using the WordPress Two-Factor plugin#13118
shervElmi wants to merge 10 commits into
developfrom
bug/11032-siwg-two-factor-account-handling

Conversation

@shervElmi

@shervElmi shervElmi commented Jul 15, 2026

Copy link
Copy Markdown
Collaborator

Summary

Addresses issue:

Relevant technical choices

  • Added a TWO_FACTOR_ENABLED_PROVIDERS_META_KEY constant for the _two_factor_enabled_providers meta key instead of repeating the literal.
  • Split the test double into its own FakeTwoFactorAuthenticator file instead of an inline subclass.
  • Placed the check in the base Authenticator, so WooCommerce_Authenticator inherits it.
  • Routed the two-factor error through the end error check in authenticate_user() instead of a duplicate one.
  • Wrote user_has_two_factor() and the test double's override as single returns.

PR Author Checklist

  • My code is tested and passes existing unit tests.
  • My code has an appropriate set of unit tests which all pass.
  • My code is backward-compatible with WordPress 5.2 and PHP 7.4.
  • My code follows the WordPress coding standards.
  • My code has proper inline documentation.
  • I have added a QA Brief on the issue linked above.
  • I have signed the Contributor License Agreement (see https://cla.developers.google.com/).

Do not alter or remove anything below. The following sections will be managed by moderators only.

Code Reviewer Checklist

  • Run the code.
  • Ensure the acceptance criteria are satisfied.
  • Reassess the implementation with the IB.
  • Ensure no unrelated changes are included.
  • Ensure CI checks pass.
  • Check Storybook where applicable.
  • Ensure there is a QA Brief.
  • Ensure there are no unexpected significant changes to file sizes.

Merge Reviewer Checklist

  • Ensure the PR has the correct target branch.
  • Double-check that the PR is okay to be merged.
  • Ensure the corresponding issue has a ZenHub release assigned.
  • Add a changelog message to the issue.

When the Two-Factor plugin is active, turn off two-factor
authentication for accounts that Sign in with Google creates, and
return an error instead of linking an existing account that already
uses two-factor. An account created with two-factor left on breaks the
later sign-in, and linking one would remove its challenge while its
password still works.
Cover the new authenticator behavior with a test double that fakes the
Two-Factor plugin state, since the test environment has no Two-Factor
plugin.
Confirm the login screen adds the two-factor message for the new error
code, and leaves an unrecognized code untouched.
@github-actions

github-actions Bot commented Jul 15, 2026

Copy link
Copy Markdown

🤖 This comment is automatically updated by CI workflows. Each section is managed independently.

🎭 Playwright reports for 83e5d1f:

📦 Build files for 83e5d1f:

Modify the assertion in the login error handling test to check for the
specific error message related to existing accounts using two-factor
authentication. This ensures that the test accurately reflects the
expected behavior when two-factor authentication is enabled.
@shervElmi
shervElmi marked this pull request as ready for review July 16, 2026 17:34
Update the `authenticate_user` method to streamline user
creation logic and enhance readability. Adjust the `user_has_two_factor`
method to simplify the conditional checks for two-factor plugin
activation.
Comment thread includes/Modules/Sign_In_With_Google/Authenticator.php Outdated
Comment thread includes/Modules/Sign_In_With_Google/Authenticator.php Outdated
Comment thread includes/Modules/Sign_In_With_Google/Authenticator.php Outdated
This change allows users to sign in with Google without being blocked
by the Two-Factor plugin's login challenge. It modifies the
Authenticator class to remove the two-factor login check during
authentication. Additionally, tests are updated to verify this
behavior for both new and returning users.
Comment thread includes/Modules/Sign_In_With_Google/Authenticator.php Outdated
Comment thread includes/Modules/Sign_In_With_Google/Authenticator.php Outdated
This change modifies the authentication process to bypass the Two-Factor
plugin's login challenge when users sign in with Google. It ensures that
the two-factor providers are cleared for the signed-in user while leaving
other users' settings intact.
Comment thread includes/Modules/Sign_In_With_Google/Authenticator.php Outdated
Comment thread includes/Modules/Sign_In_With_Google/Authenticator.php Outdated
Comment thread includes/Modules/Sign_In_With_Google/Authenticator.php Outdated
Comment on lines +336 to +338
protected function user_has_two_factor( int $user_id ) {
return $this->is_two_factor_plugin_active() && \Two_Factor_Core::is_user_using_two_factor( $user_id ); // @phpstan-ignore class.notFound (Two_Factor_Core comes from the optional Two-Factor plugin, and is_two_factor_plugin_active() confirms it exists.)
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, if we want to prevent users from signing in with their Google accounts if two-factor security is enabled and their accounts are not linked, then we need to check whether their accounts are linked to correctly validate whether the user can sign in or not.

@eugene-manuilov eugene-manuilov Jul 24, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@shervElmi, did you miss this one? If the user already has their accounts linked, we should allow them to continue signing in, right?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@eugene-manuilov I rely on the lookup order here, since find_user() matches the hashed Google ID first, so a linked 2FA user signs in fine and only an unlinked email match reaches the block.

Comment thread includes/Modules/Sign_In_With_Google.php Outdated
Comment thread includes/Modules/Sign_In_With_Google/Authenticator.php Outdated
Comment thread includes/Modules/Sign_In_With_Google.php
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants